fix: cache canvas cursor style to avoid redundant DOM writes#9171
Merged
christian-byrne merged 3 commits intomainfrom Mar 4, 2026
Merged
fix: cache canvas cursor style to avoid redundant DOM writes#9171christian-byrne merged 3 commits intomainfrom
christian-byrne merged 3 commits intomainfrom
Conversation
🎨 Storybook: ✅ Built — View Storybook |
🎭 Playwright: ✅ 541 passed, 0 failed · 8 flaky📊 Browser Reports
|
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review infoConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (3)
📝 WalkthroughWalkthroughAdds a cursor-caching utility and integrates it into LGraphCanvas so cursor updates route through a cached setter that only writes to the DOM when the cursor value changes; includes unit tests for the cache. Changes
Sequence Diagram(s)mermaid Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Comment |
christian-byrne
force-pushed
the
perf/fix-cursor-cache
branch
from
93e5404 to
873ea7d
Compare
⚡ Performance Report
Raw data{
"timestamp": "2026-02-27T03:01:34.473Z",
"gitSha": "eed317a184f9b0596a9a3fab341f2c4f1271b06a",
"branch": "perf/fix-cursor-cache",
"measurements": [
{
"name": "canvas-idle",
"durationMs": 2005.7799999999588,
"styleRecalcs": 122,
"styleRecalcDurationMs": 23.925,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 405.733,
"heapDeltaBytes": -3183896
},
{
"name": "canvas-mouse-sweep",
"durationMs": 2050.46399999992,
"styleRecalcs": 187,
"styleRecalcDurationMs": 70.30600000000001,
"layouts": 12,
"layoutDurationMs": 4.425,
"taskDurationMs": 965.042,
"heapDeltaBytes": -3424536
},
{
"name": "dom-widget-clipping",
"durationMs": 580.889999999954,
"styleRecalcs": 42,
"styleRecalcDurationMs": 13.850999999999999,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 378.611,
"heapDeltaBytes": 7577304
}
]
} |
christian-byrne
force-pushed
the
perf/fix-cursor-cache
branch
4 times, most recently
from
120f85d to
9ba9c78
Compare
christian-byrne
force-pushed
the
perf/fix-cursor-cache
branch
from
9ba9c78 to
680aee2
Compare
dosubot
bot
added
the
size:M
📦 Bundle: 4.44 MB gzip 🟢 -39 BDetailsSummary
Category Glance App Entry Points — 17.9 kB (baseline 17.9 kB) • ⚪ 0 BMain entry bundles and manifests
Status: 1 added / 1 removed Graph Workspace — 1.02 MB (baseline 1.02 MB) • ⚪ 0 BGraph editor runtime, canvas, workflow orchestration
Status: 1 added / 1 removed Views & Navigation — 72.1 kB (baseline 72.1 kB) • ⚪ 0 BTop-level views, pages, and routed surfaces
Status: 9 added / 9 removed Panels & Settings — 435 kB (baseline 435 kB) • ⚪ 0 BConfiguration panels, inspectors, and settings screens
Status: 10 added / 10 removed User & Accounts — 16 kB (baseline 16 kB) • ⚪ 0 BAuthentication, profile, and account management bundles
Status: 5 added / 5 removed Editors & Dialogs — 736 B (baseline 736 B) • ⚪ 0 BModals, dialogs, drawers, and in-app editors
Status: 1 added / 1 removed UI Components — 47.1 kB (baseline 47.1 kB) • ⚪ 0 BReusable component library chunks
Status: 5 added / 5 removed Data & Services — 2.55 MB (baseline 2.55 MB) • 🔴 +237 BStores, services, APIs, and repositories
Status: 13 added / 13 removed Utilities & Hooks — 55.5 kB (baseline 55.5 kB) • ⚪ 0 BHelpers, composables, and utility bundles
Status: 11 added / 11 removed Vendor & Third-Party — 8.84 MB (baseline 8.84 MB) • ⚪ 0 BExternal libraries and shared vendor chunks
Other — 7.77 MB (baseline 7.77 MB) • ⚪ 0 BBundles that do not match a named category
Status: 52 added / 52 removed |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/lib/litegraph/src/cursorCache.test.ts`:
- Around line 4-14: The test currently validates the standalone helper
createCursorCache() instead of exercising the production path; update the test
to instantiate a minimal LGraphCanvas (or a partial/mock instance) and call
LGraphCanvas._updateCursorStyle(), injecting a mocked canvas element whose
style.cursor setter captures assignments, then assert the setter was called only
when the cursor changes. Locate the existing test assertions around
createCursorCache and replace them with calls to LGraphCanvas._updateCursorStyle
(or a small harness that forwards to it), ensuring you reference the mocked
canvas.style.cursor setter to observe changes and keep the same change/no-change
assertions.
In `@src/lib/litegraph/src/LGraphCanvas.ts`:
- Around line 391-394: The cache _lastCursor can get out of sync because some
code writes this.canvas.style.cursor directly (e.g., the direct write found near
the code that bypasses _updateCursorStyle()); fix by centralizing cursor
updates: replace direct assignments to this.canvas.style.cursor with a single
helper (or call) that sets both this._lastCursor and this.canvas.style.cursor
(or ensure any direct write also updates this._lastCursor before changing
style); update callers that currently write the cursor directly to use the
helper or set _lastCursor so _updateCursorStyle() behavior remains correct.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/lib/litegraph/src/LGraphCanvas.tssrc/lib/litegraph/src/cursorCache.test.ts
Add a _lastCursor field to LGraphCanvas._updateCursorStyle() that tracks the last cursor value written to the DOM. Only writes canvas.style.cursor when the value has actually changed. Eliminates ~347 redundant style mutations per profiling session that were dirtying Firefox's style tree and contributing to the cascading style recalculation freeze. Includes unit test validating the caching behavior. Amp-Thread-ID: https://ampcode.com/threads/T-019c8ed0-59ad-720b-bc4f-6f52dc452844
christian-byrne
force-pushed
the
perf/fix-cursor-cache
branch
from
21c47f9 to
ec1e291
Compare
pythongosssss
approved these changes
Mar 2, 2026
Myestery
pushed a commit
that referenced
this pull request
Mar 5, 2026
## Summary Cache `canvas.style.cursor` to avoid redundant DOM writes that dirty Firefox's style tree. ## Changes - **What**: Add `_lastCursor` field to `LGraphCanvas._updateCursorStyle()` — only writes `canvas.style.cursor` when the value changes. Eliminates ~347 redundant style mutations per profiling session. ## Review Focus - The fix is 2 lines (cache field + comparison). The unit test validates the caching pattern without requiring full LGraphCanvas instantiation. - This is one of several contributors to Firefox's cascading style recalculation freeze. Each `canvas.style.cursor` write dirties the style tree, which is flushed during the next paint in the canvas render loop. ## Stack 2 of 4 in Firefox perf fix stack. Depends on #9170. <!-- Fixes #ISSUE_NUMBER --> ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-9171-fix-cache-canvas-cursor-style-to-avoid-redundant-DOM-writes-3116d73d36508139827fe1d644fa1bd0) by [Unito](https://www.unito.io)
christian-byrne
added
core/1.40
github-actions bot
pushed a commit
that referenced
this pull request
Mar 8, 2026
## Summary Cache `canvas.style.cursor` to avoid redundant DOM writes that dirty Firefox's style tree. ## Changes - **What**: Add `_lastCursor` field to `LGraphCanvas._updateCursorStyle()` — only writes `canvas.style.cursor` when the value changes. Eliminates ~347 redundant style mutations per profiling session. ## Review Focus - The fix is 2 lines (cache field + comparison). The unit test validates the caching pattern without requiring full LGraphCanvas instantiation. - This is one of several contributors to Firefox's cascading style recalculation freeze. Each `canvas.style.cursor` write dirties the style tree, which is flushed during the next paint in the canvas render loop. ## Stack 2 of 4 in Firefox perf fix stack. Depends on #9170. <!-- Fixes #ISSUE_NUMBER --> ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-9171-fix-cache-canvas-cursor-style-to-avoid-redundant-DOM-writes-3116d73d36508139827fe1d644fa1bd0) by [Unito](https://www.unito.io)
Member
|
@christian-byrne Successfully backported to #9604 |
github-actions
bot
removed
the
needs-backport
christian-byrne
added a commit
that referenced
this pull request
Mar 8, 2026
…t DOM writes (#9604) Backport of #9171 to `core/1.40` Automatically created by backport workflow. ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-9604-backport-core-1-40-fix-cache-canvas-cursor-style-to-avoid-redundant-DOM-writes-31d6d73d36508107ad71fc0ced9541e7) by [Unito](https://www.unito.io) Co-authored-by: Christian Byrne <cbyrne@comfy.org>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Cache
canvas.style.cursorto avoid redundant DOM writes that dirty Firefox's style tree.Changes
_lastCursorfield toLGraphCanvas._updateCursorStyle()— only writescanvas.style.cursorwhen the value changes. Eliminates ~347 redundant style mutations per profiling session.Review Focus
canvas.style.cursorwrite dirties the style tree, which is flushed during the next paint in the canvas render loop.Stack
2 of 4 in Firefox perf fix stack. Depends on #9170.
┆Issue is synchronized with this Notion page by Unito